Skip to content

Add error for hwmc response len 0 + replace hard-coded addresses - #285

Merged
ejgoldik merged 3 commits into
devfrom
Add_error_for_HWMC_response_len_0
Jul 16, 2025
Merged

Add error for hwmc response len 0 + replace hard-coded addresses#285
ejgoldik merged 3 commits into
devfrom
Add_error_for_HWMC_response_len_0

Conversation

@ejgoldik

Copy link
Copy Markdown
Contributor

No description provided.

@ejgoldik
ejgoldik requested review from Nir-Az and ymodlin July 16, 2025 04:59
@ymodlin
ymodlin requested a review from Copilot July 16, 2025 08:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error handling and code maintainability for the RealSense D4xx kernel driver by adding validation for zero-length HWMC responses and replacing hard-coded register addresses with predefined constants.

  • Adds error checking for zero-length HWMC responses with appropriate error logging
  • Replaces hard-coded register addresses (0x4900, 0x4904, 0x4908, 0x490c) with named constants
  • Adds explanatory comments for register operations

Comment thread kernel/realsense/d4xx.c
return -ENOBUFS;

if (tmp_len == 0) {
dev_err(&state->client->dev,

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 1827 contains a typo: "ccompletion" should be "completion".

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a mistake... I don't see this work

Comment thread kernel/realsense/d4xx.c Outdated
Comment on lines +1845 to +1846
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);

Copilot AI Jul 16, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message includes 'ret: %d' but 'ret' is always 0 at this point since the regmap_raw_read succeeded. This could be confusing as it will always log 'ret: 0' even when there's an error condition.

Suggested change
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);
"%s(): HWMC response length is 0\n",
__func__);

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret can be non-zero?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread kernel/realsense/d4xx.c Outdated
Comment on lines +1845 to +1846
"%s(): HWMC response length is 0, ret: %d\n",
__func__, ret);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret can be non-zero?

Comment thread kernel/realsense/d4xx.c
return -ENOBUFS;

if (tmp_len == 0) {
dev_err(&state->client->dev,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a mistake... I don't see this work

@Nir-Az Nir-Az left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ejgoldik
ejgoldik merged commit 4c167d7 into dev Jul 16, 2025
3 checks passed
@ymodlin
ymodlin deleted the Add_error_for_HWMC_response_len_0 branch July 16, 2025 12:47
ymodlin pushed a commit that referenced this pull request Sep 18, 2025
* Added err and dmesg in ds5_get_hwmc if response length is 0

* Added comments and replaced hard coded HWMC addresses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants